Parallelize Soroban pre-apply step. by dmkozh · Pull Request #5402 · stellar/stellar-core

dmkozh · 2026-08-04T01:05:00Z

Description

While the current logic intertwines reads and writes, in fact it can be cleanly separated into a read-only validation step, and a sequential commit step that simply bumps the sequence numbers and removes pre-authorized tx signers. This is possible because that while the writes change the entries that take part in validation, none of these changes are relevant during the validation. Specifically, sequence number bump is only observable by a single transaction (the one that has the respective account as a source), and the pre-authorized tx signer by definition belongs to a single transaction. There is also a subtle caveat to the latter operation: it increases the available balance of the signer owner (or its sponsor), but since at the pre-apply time the fees have already been charged, we're only checking that the account available balance is non-negative, which is an invariant that must always hold in the current protocol.

The change is not protocol-gated because it's not a protocol change for the current protocol. It was technically a protocol change prior to p26 where we had a bug that actually did allow overcharging the fee bump source accounts and thus making their available balance to go negative. However, the bug has been fixed without the behavior ever triggering on-chain, and thus this replay-only behavior change should be non-observable.

This change significantly speeds up the pre-apply step. On the local high TPL benchmarks I'm getting 30-60ms improvement locally compared to the main branch version.

This PR also adds test coverage for the edge case pre-apply interactions that weren't covered well (or at all), such as pre-auth signer removal, tx invalidation due to sequence bumps etc. This also contains a randomized acceptance test that ensures that parallel pre-apply logic is deterministic across any number of worker threads.

Checklist

Reviewed the contributing document
Rebased on top of master (no merge commits)
Ran clang-format v8.0.0 (via make format or the Visual Studio extension)
Compiles
Ran all tests
If change impacts performance, include supporting evidence per the performance document

Copilot

Pull request overview

Parallelizes Soroban pre-apply validation while preserving serial ledger mutations.

Changes:

Splits pre-apply into parallel read-only and sequential write phases.
Adds a ledger view combining classic-phase changes with LCL state.
Adds concurrency and signer/sequence regression tests with updated baselines.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`test-tx-meta-baseline-next/InvokeHostFunctionTests.json`	Updates next-protocol test baselines.
`test-tx-meta-baseline-current/InvokeHostFunctionTests.json`	Updates current-protocol test baselines.
`src/util/BatchExecutor.h`	Exposes preferred parallelism controls.
`src/util/BatchExecutor.cpp`	Computes preferred worker count.
`src/transactions/TransactionFrameBase.h`	Splits the pre-apply interface.
`src/transactions/TransactionFrame.h`	Declares read/write pre-apply paths.
`src/transactions/TransactionFrame.cpp`	Implements separated pre-apply processing.
`src/transactions/test/TransactionTestFrame.h`	Adapts the test transaction interface.
`src/transactions/test/TransactionTestFrame.cpp`	Delegates split pre-apply calls.
`src/transactions/test/SorobanTxTestUtils.cpp`	Corrects footprint deduplication.
`src/transactions/test/ParallelApplyTest.cpp`	Tests worker-count independence.
`src/transactions/test/InvokeHostFunctionTests.cpp`	Adds pre-apply regression coverage.
`src/transactions/ParallelApplyUtils.h`	Declares parallel pre-apply orchestration.
`src/transactions/ParallelApplyUtils.cpp`	Runs parallel reads and serial writes.
`src/transactions/FeeBumpTransactionFrame.h`	Splits fee-bump pre-apply APIs.
`src/transactions/FeeBumpTransactionFrame.cpp`	Implements fee-bump pre-apply phases.
`src/ledger/ImmutableLedgerView.h`	Defines the pre-apply ledger view.
`src/ledger/ImmutableLedgerView.cpp`	Implements combined ledger lookups.

Suppressed comments (3)

src/transactions/ParallelApplyUtils.cpp:442

Cap the task count at the number of bundles. BatchExecutor::ensureWorkers creates and retains one worker per submitted task, so a small ledger on a many-core host currently starts dozens of workers whose ranges are empty, adding startup and wake-up overhead to the path this PR is optimizing.

    size_t taskCount = app.getBatchExecutor().preferredTaskCount();

src/transactions/ParallelApplyUtils.cpp:400

This rationale has the direction backwards: removing a signer decreases the owner's sub-entry count (and, when sponsored, the sponsor's sponsoring count), which can only increase available balance. State that monotonic effect explicitly because it is the reason deferring these writes cannot invalidate another transaction.

    // - The removal of one-time pre-authorized tx signers - this is also fine
    //   because any given transaction in a ledger is unique, and increasing the
    //   sub-entry count of a source/sponsor account is not relevant at that
    //   point, as the fees have already been successfully charged.

src/transactions/ParallelApplyUtils.cpp:391

Correct the typo “separatation” to “separation”.

    // This phase separatation hinges on the fact that the validation outcome

Copilot

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

Suppressed comments (1)

src/util/BatchExecutor.cpp:226

Ceiling the range size does not reliably create numTasks ranges: for example, count=4, numTasks=3 creates only two tasks. This underutilizes available cores in the performance-sensitive path and contradicts the API contract. Distribute the remainder across exactly numTasks ranges instead.

    auto rangeSize = (count + numTasks - 1) / numTasks;
    std::vector<std::function<int()>> tasks;
    tasks.reserve(numTasks);
    for (size_t begin = 0; begin < count; begin += rangeSize)

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/util/BatchExecutor.cpp:223

This ceiling-division expression can overflow size_t. For example, count == SIZE_MAX and numTasks == 2 produces rangeSize == 0, so the following loop never advances; even with a safe quotient, begin + rangeSize also needs overflow protection. Compute the quotient/remainder without addition overflow and advance via a clamped end.

    auto rangeSize = (count + numTasks - 1) / numTasks;

While the current logic intertwines reads and writes, in fact it can be cleanly separated into a read-only validation step, and a sequential commit step that simply bumps the sequence numbers and removes pre-authorized tx signers. This is possible because that while the writes change the entries that take part in validation, none of these changes are relevant during the validation. Specifically, sequence number bump is only observable by a single transaction (the one that has the respective account as a source), and the pre-authorized tx signer by definition belongs to a single transaction. There is also a subtle caveat to the latter operation: it increases the available balance of the signer owner (or its sponsor), but since at the pre-apply time the fees have already been charged, we're only checking that the account available balance is non-negative, which is an invariant that must always hold in the current protocol. The change is not protocol-gated because it's not a protocol change for the *current* protocol. It was technically a protocol change prior to p26 where we had a bug that actually did allow overcharging the fee bump source accounts and thus making their available balance to go negative. However, the bug has been fixed without the behavior ever triggering on-chain, and thus this replay-only behavior change should be non-observable. This change significantly speeds up the pre-apply step. On the local high TPL benchmarks I'm getting 30-60ms improvement locally compared to the main branch version.

Copilot

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/transactions/FeeBumpTransactionFrame.cpp:122

The new parallel write path removes the fee-source pre-auth signer before delegating to the inner transaction, but the added Soroban tests only fee-bump transactions whose fee source has no pre-auth signer; existing one-time-signer coverage exercises the old sequential apply path. Please add a p23+ parallel test that installs the fee-bump hash as a signer on the fee source (including sponsorship if supported) and verifies its removal and metadata.

            LedgerTxn ltxTx(ltx);
            removeOneTimeSignerKeyFromFeeSource(ltxTx);
            meta.pushTxChangesBefore(ltxTx);
            ltxTx.commit();
        }
        mInnerTx->preParallelApplyWrite(app, ltx, meta, txResult);

Note

Copilot is running an experiment and ran this review at Lite.

Copilot AI balanced review requested due to automatic review settings August 4, 2026 01:05

Copilot started reviewing on behalf of dmkozh August 4, 2026 01:05 View session

dmkozh force-pushed the par_pre_apply branch 2 times, most recently from b7896a9 to be994a5 Compare August 4, 2026 01:07

Copilot AI reviewed Aug 4, 2026

View reviewed changes

dmkozh force-pushed the par_pre_apply branch 2 times, most recently from 6f7c2f1 to 4cdc91b Compare August 4, 2026 18:18

dmkozh force-pushed the par_pre_apply branch 2 times, most recently from d7c7629 to 02b730f Compare August 14, 2026 20:10

dmkozh requested review from graydon and sisuresh August 14, 2026 20:49

sisuresh reviewed Sep 8, 2026

View reviewed changes

Comment thread src/transactions/TransactionFrame.cpp

dmkozh force-pushed the par_pre_apply branch from 02b730f to 41f8b63 Compare September 9, 2026 23:21

Copilot AI review requested due to automatic review settings September 9, 2026 23:21

Copilot started reviewing on behalf of dmkozh September 9, 2026 23:21 View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Comment thread src/util/BatchExecutor.cpp

Comment thread src/transactions/ParallelApplyUtils.cpp

Comment thread src/util/BatchExecutor.h

Copilot AI review requested due to automatic review settings September 10, 2026 01:14

dmkozh force-pushed the par_pre_apply branch from 41f8b63 to 1d2b7d4 Compare September 10, 2026 01:14

Copilot started reviewing on behalf of dmkozh September 10, 2026 01:14 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Comment thread src/util/BatchExecutor.cpp

Comment thread src/transactions/test/InvokeHostFunctionTests.cpp Outdated

Comment thread src/transactions/test/InvokeHostFunctionTests.cpp Outdated

Copilot AI review requested due to automatic review settings September 10, 2026 16:57

dmkozh force-pushed the par_pre_apply branch from 1d2b7d4 to 7d5a01b Compare September 10, 2026 16:57

Copilot started reviewing on behalf of dmkozh September 10, 2026 16:58 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Comment thread src/transactions/ParallelApplyUtils.cpp

Copilot AI review requested due to automatic review settings September 11, 2026 17:41

dmkozh force-pushed the par_pre_apply branch from 7d5a01b to 8958c27 Compare September 11, 2026 17:41

Copilot started reviewing on behalf of dmkozh September 11, 2026 17:41 View session

Copilot AI reviewed Sep 11, 2026

View reviewed changes

Navigation Menu

Uh oh!

Parallelize Soroban pre-apply step. - #5402

Parallelize Soroban pre-apply step.#5402
dmkozh wants to merge 1 commit into
stellar:masterfrom
dmkozh:par_pre_apply

dmkozh commented Aug 4, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

dmkozh commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dmkozh commented Aug 4, 2026 •

edited

Loading